MacOS getDevice() optimization, set CI env var, & add getVideoDevices() test#1691
MacOS getDevice() optimization, set CI env var, & add getVideoDevices() test#1691sandboxcoder wants to merge 2 commits into
Conversation
3a0c9ba to
a935b64
Compare
There was a problem hiding this comment.
Pull request overview
This PR reintroduces and refines a getDevices optimization to avoid creating dummy sources (notably preventing CoreAudio from spinning up unnecessary reconnect threads), adds a macOS-specific video device enumeration path to avoid plugin initialization requirements, and updates CI/test utilities to better handle Darwin CI constraints.
Changes:
- Replace dummy-source-based property enumeration with
obs_get_source_properties()for device listing, and add warning logs on property lookup failures. - Implement manual macOS video device enumeration via AVFoundation and route
OBS_settings_getVideoDevices(macOS) through it. - Add/adjust OSN tests and CI flags to better detect Darwin CI and validate video device enumeration output shape.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/osn-tests/util/obs_handler.ts | Adds isOnDarwinCI() helper for test gating on macOS CI. |
| tests/osn-tests/util/error_messages.ts | Adds new error messages for video device enumeration tests. |
| tests/osn-tests/src/test_osn_video.ts | Adds a test for OBS_settings_getVideoDevices() return shape and (non-CI) non-empty expectation. |
| tests/osn-tests/src/test_osn_audio.ts | Gates default output device expectation behind isOnDarwinCI(). |
| obs-studio-server/source/nodeobs_settings.cpp | Updates getDevices() to avoid dummy source creation; macOS video devices now enumerated via new macOS helper. |
| obs-studio-server/source/nodeobs_settings-osx.mm | New AVFoundation-based macOS video device enumeration implementation. |
| obs-studio-server/source/nodeobs_settings-osx.h | Declares macOS video device enumeration helper. |
| obs-studio-server/CMakeLists.txt | Adds new macOS settings enumeration sources to the build. |
| .github/workflows/main.yml | Explicitly sets CI=true for the macOS test job environment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This reverts commit 4434fda.
32817d2 to
8b48df9
Compare
65e6812 to
9b7e04f
Compare
| # Comfigure macos architecture and min version | ||
| iF(APPLE) | ||
| set(CMAKE_XCODE_ATTRIBUTE_MACOSX_DEPLOYMENT_TARGET[arch=arm64] "11.0") | ||
| set(CMAKE_XCODE_ATTRIBUTE_MACOSX_DEPLOYMENT_TARGET[arch=x86_64] "10.15") | ||
| set(CMAKE_XCODE_ATTRIBUTE_MACOSX_DEPLOYMENT_TARGET[arch=arm64] "12.0") | ||
| set(CMAKE_XCODE_ATTRIBUTE_MACOSX_DEPLOYMENT_TARGET[arch=x86_64] "12.0") | ||
| if (NOT CMAKE_OSX_ARCHITECTURES) |
There was a problem hiding this comment.
Yes, Streamlabs Desktop minimum version is actually MacOS 12 (this version in particular is special, my minspec Intel laptop is set to MacOS 12 while the Mac M4 is using latest). I just never updated the deployment target in the xcode cmake file because we never needed to bump it (only reason I was thinking to bump it now is to make it obvious there is no need for fallback code for anything older then macOS 12) but should have bumped this much earlier...
This matches SLOBS Deployment target now.
72cd389 to
d4537f7
Compare
…es() * rename legacy getDevices() -> getDevicesUsingDummySource(). Adding more error checking * set CI flag for MacOS test runner so it can use obs.isCI()
6623119 to
d56cf32
Compare
Description
reconnect_threadwhenever Desktop queries for audio devices. This was because the previous implementation passed in a dummy device which causes coreaudio to create a thread to poll if this dummy device is later initialized. This will cleanup the logs eliminating this warning (32 occurrences in a log I checked from Desktop):[Warning] [coreaudio_get_device_name] failed to get name: 560947818obs.isCI()function to determine if we're running in a CI env (no GPU) very quickly from the tests.OBS_settings_getVideoDevices.disable on CI runner
CMAKE_OSX_DEPLOYMENT_TARGETto MacOS 12. now matches SLOBS which is set herePerformance:
New implementation (nanoseconds)
Old (nanoseconds)
The new
enumDevicesimplementation is more efficient since it does not create adummy_source. Also, it avoids triggering coreaudio to create a reconnect_thread for a dummy device (which saves us another 2ms or so in an async thread because mac-coreaudio polls within the reconnect_thread).Motivation and Context
When I was looking into profiling missing source devices it reminded me that getDevices() triggers coreaudio to create a reconnect_thread which does later get reaped however it did not seem ideal. But it would be nice to avoid this. Currently, desktop queries getAudioDevices() at least around 14 times so it is nice to optimize this to get the app to load up faster and spin up less threads.
How Has This Been Tested?
Locally, I verified the same audio/video devices are output. Originally, commit 9279b8a was reverted due to breaking the old onboarding flow (no webcam detected) but that is addressed in this PR since we will enumerate video devices using the legacy approach.
Types of changes
Checklist: